This unconditionally prefers the host assembly by simple name, even when the package closure selected a newer/incompatible version. That can recreate the same MissingMethodException class of failure this change is intended to fix—for example, the PR notes that Extensions requests System.Memory.Data 10.0.0.9 while the host carries 10.0.0.3. If maintaining a single type identity requires using the host copy, please verify compatibility with the closure requirement and report a clear resolution failure when it is not compatible rather than silently substituting it.

--generated by Copilot

Addressed, though not by failing. Made the substitution visible instead.

UseHostAssembly now compares the host's version against the requested one and records a note when the host's copy is older. DescribeDowngradedDependencies() is appended to the "could not be loaded" and "does not declare the type" failure reasons, so a type that fails because of the substitution now says so rather than looking like a plain missing type.

I deliberately did not make the mismatch fatal, because there is no version-compatible answer available when the host and the closure disagree:

• Load the closure's copy: two assemblies with the same simple name end up in the process. System.Memory.Data declares System.BinaryData rather than forwarding it, so BinaryData then exists twice and every signature mentioning it fails to bind. That is exactly the MissingMethodException this PR was written to fix.
• Return null: the load fails outright with Could not load file or assembly 'System.Memory.Data, Version=10.0.0.9'. I tried this variant; it regresses the working scenario.
• Use the host's copy: one identity per simple name, and in practice it works, because assembly versions are routinely older than the reference that names them.

The concrete case here is the generator deploying System.Memory.Data 10.0.0.3 while Azure.AI.Extensions.OpenAI asks for 10.0.0.9. Both are BinaryData-over-corlib shims; the request is only unsatisfiable because the .NET SDK normally prunes this package on net10.0 and the generator carries it explicitly.

Note also that the hook only ever runs for names the default context could not satisfy on its own. It cannot prevent that same context from satisfying other, lower-versioned requests for the same name from its own TPA list, so declining here does not buy isolation - it just loses the load.

This regression test does not exercise RegisterPackageClosure/WalkPackage: FakeNuGetPackage.Create writes only the DLL under lib/netstandard2.0 and does not create a .nuspec, so closure walking immediately takes the missing-nuspec fallback. Because both fake package and assembly versions are also 1.0.0, the test passes through the assembly-version fallback. Please add a dependency group to a fake .nuspec and deliberately make the dependency package version differ from its assembly version (for example package 2.0.0 containing assembly 1.0.0.0) so the new closure/version-selection path is actually required for the test to pass.

--generated by Copilot

To clarify the requested scope: please add only the hermetic unit test described above. We can leave validation against real published NuGet packages as a manual E2E check for now.

--generated by Copilot

Agreed, and that is how it was done. The test builds its packages on disk under a temp NUGET_PACKAGES folder with Roslyn and never contacts a feed. The real Azure.AI.Extensions.OpenAI / OpenAI / System.ClientModel graph stays a manual end-to-end check.

Good catch, and correct. FakeNuGetPackage.Create wrote only lib/netstandard2.0/{pkg}.dll and no .nuspec, so WalkPackage hit its missing-nuspec early return, _closureVersions stayed empty, and the test passed entirely through the assembly-version fallback. It was not testing the code it was written for.

Fixed by emitting a real .nuspec with a .NETStandard2.0 dependency group, and giving the test helper an assemblyVersion parameter separate from the package version. The test now builds:

• Test.Dependency.Base package 2.0.0 / assembly 1.0.0.0, declaring DependencyBaseType
• Test.Dependency.Base package 3.0.0 / assembly 1.0.0.0, a decoy without that type
• Test.Dependent.Leaf 1.0.0, whose .nuspec depends on Test.Dependency.Base [2.0.0, )

The leaf's assembly reference carries version 1.0.0.0, so the fallback searches for the highest package at or above 1.0.0 and lands on the 3.0.0 decoy. Only the .nuspec closure identifies 2.0.0.

Verified by neutering RegisterPackageClosure and re-running:

Failed TryResolve_ResolvesTypeWhoseBaseTypeLivesInAnotherPackage
  Could not load type 'Test.Dependency.Base.DependencyBaseType' from assembly
  'Test.Dependency.Base, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'

Closure walking is now strictly required for the test to pass.

Keeping assembly resolution as a separate type makes sense, but can this be a per-generator instance owned by ExternalTypeReferenceResolver's cache state rather than process-wide static state? The closure versions, resolved assemblies, in-progress set, load-context hook, and debug callback all belong to one generation run. Instance ownership would prevent state/type-binding decisions from leaking between sequential generator invocations and would make teardown and isolated testing explicit instead of relying on every static collection and event subscription being reset correctly.

--generated by Copilot

Done. NugetAssemblyResolver is an internal sealed class instantiated lazily by ExternalTypeReferenceResolver's per-generator CacheState, so it shares that state's lifetime and is dropped by Reset(). _resolved, _closureVersions, _walkedPackages and _registeredReferences are all instance fields now, [ThreadStatic] became ThreadLocal<HashSet<string>>, and the debug sink and metadata-reference registration are injected as delegates rather than reaching for CodeModelGenerator.Instance.

Two things stay static, because they are genuinely process-wide rather than per-run:

• AssemblyLoadContext.Default exposes exactly one Resolving event. A single handler is installed on first use and dispatches to Volatile.Read(ref _active); Activate() / Deactivate() swap the target. Subscribing per instance would be a leak - CacheState lives in a ConditionalWeakTable with no deterministic disposal, so handlers would accumulate across runs in one process, which is the norm under test.
• Assemblies loaded into the default context can never be unloaded, so Deactivate() only detaches the bookkeeping.

This is safe because FindHostAssembly reads process truth directly from AssemblyLoadContext.Default.Assemblies rather than mirroring it in a static. The instance dictionaries are therefore pure caches, and a fresh instance re-derives the same answers.

Have we considered re-utilize the existing TryFindPackageInCache method in NugetPackageDownloader ? It calls the nuget SDK to delegate this search to their library vs having to handwrite it ourselves.

This looks a bit odd. If this happened in a unit test, should we ensure the generator is initialized as part of the test setup instead?

it would be great if we can add unit tests for this class and mock some of these methods to ensure things work as expected, and also ensure there are no unexpected crashes if things don't. Assembly loading is hard to get right in my experience 🙂 . Feel free to ignore if you feel the existing tests you added cover failure scenarios.